Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

v1.4.0 (CosmWasm v1) #930

Merged
merged 577 commits into from
Sep 12, 2022
Merged

v1.4.0 (CosmWasm v1) #930

merged 577 commits into from
Sep 12, 2022

Conversation

assafmo
Copy link
Member

@assafmo assafmo commented May 15, 2022 •
edited by liorbond
Loading

  • In ContractInstance/start_engine(), check whether the wasm is v0.10 or v1 (using this export) and store it in ContractInstance, then (after the crypto stuff) according to the cosmwasm version, initialize the env object that is given to the contract in:
    • init
    • handle
    • query (in v1 query has env too).
      • Pass env from Go to query.
      • In query, don't need to pass contract_key prefixed to msg anymore, because it's also inside env.
  • Also, invoke the right wasm function:
    • v0.10 init or v1 instantiate (link).
    • v0.10 handle or v1 execute (link)
  • Afterwards, translate the v1 output into v0.10 output so that the Go code will handle it nicely.
    • Add plaintext_log() to cosmwasm-std v1
    • Handle v1 submessages. TODO: See how cosmwasm implemented this, we might want to piggyback off messages and handle it in the Go code, or maybe we can handle it right there inside the enclave by converting them to normal messages.
  • Fix wasm validation. The new v1 API needs these while we currently have the v0.10 API which validates these. In order to support both in parallel, we need to fix the validation code.
  • Implement new v1 API
    • db_read - we already have this from v0.10
    • db_write - we already have this from v0.10
    • db_remove - we already have this from v0.10
    • addr_validate
    • addr_canonicalize - need to alias from canonicalize_address (v0.10)
    • addr_humanize - need to alias from humanize_address (v0.10)
    • secp256k1_verify
    • secp256k1_recover_pubkey
    • ed25519_verify
    • ed25519_batch_verify
    • ed25519_sign - we've added on top on v1
    • secp256k1_sign - we've added on top on v1
    • debug - debug prints from inside the contract
    • query_chain - we already have this from v0.10
    • Charge gas for all functions that don't go back into the chain (Basically the addr & crypto ones).
  • New contract entry points
    • ibc_channel_open()
    • ibc_channel_connect()
    • ibc_channel_close()
    • ibc_packet_receive()
    • ibc_packet_ack()
    • ibc_packet_timeout()
    • reply()
  • Implement migration for cotnractInfo due to the new changes (added IBC port for each contract)
  • When handling submessages of v0.10, make sure that errors when ReplyOn::Never fails the tx (because in v0.10 errors on submessages always failed the tx) - Code-Wise we saw that the behavior is as expected but a test need to be written
  • Add Withdraw handler to v1 StakingMsg
  • Handler callback_sig for v1 result inside the enclave
  • Audit StargateQuery's whitelist
  • Audit StargateMsg
  • Test the data field - v1 lets replies override the data field, while v0.10 always returns the first message's data field.
  • Initialize Env.TransactionInfo

@assafmo assafmo linked an issue Jul 13, 2022 that may be closed by this pull request
Implement CosmWasm v1 #770
Closed
@faddat
Copy link
Collaborator

faddat commented Jul 14, 2022 •
edited
Loading

So, I did this

and

but depends on if ya'd like to get freaky or nah I suppose?

:P

downside: grpc only
upside: grpc only
upside: mempool prioritization

@assafmo
Copy link
Member Author

assafmo commented Jul 14, 2022

So, I did this

and

but depends on if ya'd like to get freaky or nah I suppose?

:P

downside: grpc only upside: grpc only upside: mempool prioritization

We do want it, also has gov and authz goodies. But first we just need to check with all exchanges that they're not using the old REST API.

@assafmo
Copy link
Member Author

assafmo commented Jul 14, 2022

But yeah please base new work on the cosmwasm-v1 branch ❤️

@faddat
Copy link
Collaborator

faddat commented Jul 15, 2022

About the API, I think that these breaking changes are worth exploring because I think others will have similar concerns. It is my understanding that the GRPC gateway could get us around this but I don't know how accurate that is.

@assafmo
Copy link
Member Author

assafmo commented Jul 17, 2022

It is my understanding that the GRPC gateway could get us around this but I don't know how accurate that is.

Yes that's accurate.

@assafmo assafmo mentioned this pull request Jul 20, 2022
Closed
@faddat
Copy link
Collaborator

faddat commented Jul 22, 2022 •
edited
Loading

OK, so basically -- if we're fundamentally good with "we won't break exchange API's if go to 46" -- I'd like to either invite you guys to the Notional slack server, or to a call or something.

We're doing 46 on numerous chains and I'd love for secret to be one of them.

@assafmo
Copy link
Member Author

assafmo commented Jul 22, 2022

OK, so basically -- if we're fundamentally good with "we won't break exchange API's if go to 46" -- I'd like to either invite you guys to the Notional slack server, or to a call or something.

We're doing 46 on numerous chains and I'd love for secret to be one of them.

We'd love to open a Slack connect! Our Slack org name is enigmaco.

We're still reaching out to exchanges and we'll know for sure in a few weeks. Our next chain upgrade is planned for September, so there's some time before we need to make a decision re 45->46.

Cc @YonatanE22

liorbond and others added 14 commits August 28, 2022 23:48
@liorbond
Channel close tests
4058c9e
@liorbond
Merge remote-tracking branch 'origin/cw-1-ibc-receive-hooks' into esh…
c32ba0b 
…el-ibc
@eshelB
adding util file that lets me see a transaction more cleanly: convert…
a0b0bba 
…ing attributes from the form of bytes to their string form
@liorbond
IBC packet receive tests and bag fixes
b49d664
@liorbond
Merge pull request #1096 from scrtlabs/lior-ibc-tests
498d961 
IBC packet receive tests and bag fixes
@liorbond
IBC ack and timeout should be encrypted
7d0f85a
@liorbond
IBC ack tests
b058d3c
@liorbond
Encrypt the output of every reply (Should be changed)
5b97e6f
@eshelB
adding util file that lets me see a transaction more cleanly: convert…
8152de2 
…ing attributes from the form of bytes to their string form
@liorbond
Encrypt plaintext replies only if the original message was encrypted
7d65399
@liorbond
Merge pull request #1097 from scrtlabs/lior-ibc-tests
b7ab07f 
All IBC tests
@eshelB
Merge branch 'eshel-ibc' of github.com:scrtlabs/SecretNetwork into es…
dfd161b 
…hel-ibc
@liorbond
Fix gtests
add2302
@liorbond
Merge pull request #1098 from scrtlabs/lior-ibc-tests
50551f0 
Fix gtests
liorbond
liorbond previously approved these changes Sep 8, 2022
View reviewed changes
@assafmo assafmo changed the title CosmWasm v1 v1.4.0 (CosmWasm v1) Sep 11, 2022
@Cashmaney Cashmaney merged commit 9e3861e into master Sep 12, 2022
@assafmo assafmo deleted the cosmwasm-v1 branch September 12, 2022 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cashmaney Cashmaney Cashmaney approved these changes

@liorbond liorbond liorbond left review comments

@reuvenpo reuvenpo Awaiting requested review from reuvenpo

@toml01 toml01 Awaiting requested review from toml01 toml01 is a code owner

Assignees

@assafmo assafmo

@liorbond liorbond

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement CosmWasm v1
8 participants
@assafmo @faddat @Cashmaney @liorbond @eshelB @gjermundgaraba @reuvenpo @toml01